fix: UI polish — settings, chat, overscroll, session badges#32
Conversation
- Remove duplicate "Check Permissions" button in WhisperSettings - Make settings tabs full-width with equal distribution - Remove "Settings" text from window title bar - Add session count badge on workspace cards - Disable macOS rubber-band overscroll - Fix terminal chat dot alignment with text baseline - Close review panel on plan approval - Set min-width on Send/Stop button to prevent layout shift
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds backend session-counting commands and exposes them to the Tauri frontend; the HomeScreen fetches per-workspace session counts and passes them to WorkspaceCard for badge rendering. Also includes minor UI styling and settings window title changes. Changes
Sequence DiagramsequenceDiagram
participant HS as HomeScreen
participant Tauri as Tauri Layer
participant DB as Database
participant WC as WorkspaceCard
HS->>Tauri: invoke("count_workspace_sessions", workspace_paths)
Tauri->>DB: count_sessions_batch(conn, workspace_paths)
DB-->>Tauri: Vec<(path, count)>
Tauri-->>HS: Result<Vec<(String,i64)>, String>
HS->>HS: set sessionCounts state
HS->>WC: render WorkspaceCard(sessionCount)
WC->>WC: show session badge if sessionCount > 0
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/tauri/src/components/MarkdownViewer.tsx (1)
200-207:⚠️ Potential issue | 🟠 Major
setIsPanelOpen(false)alone won’t close the visible panelLine 205 updates review state, but this component collapses the UI via
reviewRef.current?.collapse()(andonCollapse). With current code, approve can leave the panel visually open, missing the PR objective.Suggested fix
const handleApprove = useCallback(() => { if (!onApprovePlan) return; const reviewText = review.generateReview(); const hasComments = review.comments.some((c) => !c.resolved); review.resolveAll(); - review.setIsPanelOpen(false); + reviewRef.current?.collapse(); + review.setIsPanelOpen(false); onApprovePlan(hasComments ? reviewText : undefined); }, [review.generateReview, review.comments, review.resolveAll, review.setIsPanelOpen, onApprovePlan]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/MarkdownViewer.tsx` around lines 200 - 207, The approve handler (handleApprove) updates review state via review.setIsPanelOpen(false) but does not actually collapse the visible panel; call the UI collapse method to ensure the panel is closed visually. Modify handleApprove to invoke reviewRef.current?.collapse() (or call onCollapse()) after resolving comments and before/after setIsPanelOpen(false), and add reviewRef (or onCollapse) to the hook dependencies so the ref/callback is referenced correctly; ensure you null-check reviewRef.current?.collapse before calling it.
🧹 Nitpick comments (3)
apps/tauri/src-tauri/src/sessions.rs (1)
177-191: Consider batching queries for efficiency.The current implementation executes N separate queries (one per workspace path). For typical usage with a small number of workspaces, this is fine. However, if the workspace count grows, a single query with an
INclause would be more efficient.♻️ Optional: Single-query approach
pub fn count_sessions_batch(conn: &Connection, workspace_paths: &[String]) -> Result<Vec<(String, i64)>, String> { - let mut results = Vec::with_capacity(workspace_paths.len()); - let mut stmt = conn - .prepare("SELECT COUNT(*) FROM sessions WHERE workspace_path = ?1") - .map_err(|e| format!("Prepare error: {}", e))?; - for path in workspace_paths { - let count: i64 = stmt - .query_row(params![path], |row| row.get(0)) - .unwrap_or(0); - if count > 0 { - results.push((path.clone(), count)); - } - } - Ok(results) + if workspace_paths.is_empty() { + return Ok(Vec::new()); + } + let placeholders: Vec<_> = (1..=workspace_paths.len()).map(|i| format!("?{}", i)).collect(); + let sql = format!( + "SELECT workspace_path, COUNT(*) FROM sessions WHERE workspace_path IN ({}) GROUP BY workspace_path HAVING COUNT(*) > 0", + placeholders.join(", ") + ); + let mut stmt = conn.prepare(&sql).map_err(|e| format!("Prepare error: {}", e))?; + let params: Vec<&dyn rusqlite::ToSql> = workspace_paths.iter().map(|p| p as &dyn rusqlite::ToSql).collect(); + let rows = stmt + .query_map(params.as_slice(), |row| Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?))) + .map_err(|e| format!("Query error: {}", e))? + .collect::<Result<Vec<_>, _>>() + .map_err(|e| format!("Row error: {}", e))?; + Ok(rows) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/sessions.rs` around lines 177 - 191, The current count_sessions_batch runs one query per workspace_path; change it to issue a single SQL query using "SELECT workspace_path, COUNT(*) FROM sessions WHERE workspace_path IN (?1,?2,...) GROUP BY workspace_path" to improve performance: in count_sessions_batch build dynamic placeholders matching workspace_paths.len(), return early with empty vec if workspace_paths is empty, prepare that single statement (instead of the existing per-path stmt), bind all workspace_paths as params, iterate over rows to push (workspace_path, count) into results, and preserve the same Result<Vec<(String,i64)>, String> error handling.apps/tauri/src/components/HomeScreen.tsx (1)
38-51: Dependency array pattern creates unnecessary closures.The
dirWorkspaces.map((w) => w.path).join(",")in the dependency array (line 51) is evaluated on every render before comparison. While the resulting string is stable when paths don't change, the intermediate.map()call allocates a new array each time. Consider memoizing the paths or moving the filtering logic inside the callback.The same pattern exists in
fetchCommentCounts(line 36). Both could be simplified:♻️ Suggested improvement using useMemo
+ const dirPaths = useMemo(() => dirWorkspaces.map((w) => w.path), [dirWorkspaces]); + const filePaths = useMemo(() => fileWorkspaces.map((w) => w.path), [fileWorkspaces]); const fetchCommentCounts = useCallback(async () => { - if (fileWorkspaces.length === 0) return; - const paths = fileWorkspaces.map((w) => w.path); + if (filePaths.length === 0) return; try { - const results = await invoke<[string, number][]>("count_unresolved_comments", { filePaths: paths }); + const results = await invoke<[string, number][]>("count_unresolved_comments", { filePaths }); // ... } catch { // silently ignore } - }, [fileWorkspaces.map((w) => w.path).join(",")]); + }, [filePaths]); const fetchSessionCounts = useCallback(async () => { - if (dirWorkspaces.length === 0) return; - const paths = dirWorkspaces.map((w) => w.path); + if (dirPaths.length === 0) return; try { - const results = await invoke<[string, number][]>("count_workspace_sessions", { workspacePaths: paths }); + const results = await invoke<[string, number][]>("count_workspace_sessions", { workspacePaths: dirPaths }); // ... } catch { // silently ignore } - }, [dirWorkspaces.map((w) => w.path).join(",")]); + }, [dirPaths]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/HomeScreen.tsx` around lines 38 - 51, The dependency array currently calls dirWorkspaces.map(...).join(",") which allocates on every render and causes unnecessary closures in fetchSessionCounts (and similarly fetchCommentCounts); fix by memoizing the derived paths with useMemo (e.g. const workspacePaths = useMemo(() => dirWorkspaces.map(w => w.path), [dirWorkspaces]) and then use workspacePaths (or workspacePaths.join(",")) in the dependency arrays, or alternatively move the mapping inside fetchSessionCounts and depend directly on dirWorkspaces to avoid the repeated map allocation. Ensure you update both fetchSessionCounts and fetchCommentCounts to reference the memoized workspacePaths or dirWorkspaces consistently.apps/tauri/src/components/WorkspaceCard.tsx (1)
86-98: Badge positioning relies on mutual exclusivity.Both badges use identical
absolute top-1.5 right-1.5positioning. Currently this works becauseunresolvedCommentsis only passed to file workspaces andsessionCountto directory workspaces (they're mutually exclusive). However, this creates a maintenance hazard if future changes allow both badges on the same card.Consider making this explicit with a comment, or restructuring to handle potential co-existence:
💡 Option: Add clarifying comment
+ {/* Note: unresolvedComments and sessionCount are mutually exclusive by workspace type */} {unresolvedComments != null && unresolvedComments > 0 && ( <span className="absolute top-1.5 right-1.5 inline-flex items-center gap-0.5 text-muted-foreground group-hover:opacity-0 transition-opacity">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/components/WorkspaceCard.tsx` around lines 86 - 98, The two badges (unresolvedComments and sessionCount) in WorkspaceCard share identical absolute positioning ("absolute top-1.5 right-1.5") which will cause overlap if both become present; update WorkspaceCard to either (a) render a single top-right badge container and place both badges inside it using a vertical or horizontal layout with spacing (e.g., a flex column/row with gap) so they don't collide, or (b) give each badge distinct offsets (e.g., top-1.5 right-1.5 and top-1.5 right-6) so they stack, and add a brief inline comment above the badge rendering explaining the choice; ensure you reference and conditionally render unresolvedComments and sessionCount inside that adjusted container.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/tauri/src/components/MarkdownViewer.tsx`:
- Around line 200-207: The approve handler (handleApprove) updates review state
via review.setIsPanelOpen(false) but does not actually collapse the visible
panel; call the UI collapse method to ensure the panel is closed visually.
Modify handleApprove to invoke reviewRef.current?.collapse() (or call
onCollapse()) after resolving comments and before/after setIsPanelOpen(false),
and add reviewRef (or onCollapse) to the hook dependencies so the ref/callback
is referenced correctly; ensure you null-check reviewRef.current?.collapse
before calling it.
---
Nitpick comments:
In `@apps/tauri/src-tauri/src/sessions.rs`:
- Around line 177-191: The current count_sessions_batch runs one query per
workspace_path; change it to issue a single SQL query using "SELECT
workspace_path, COUNT(*) FROM sessions WHERE workspace_path IN (?1,?2,...) GROUP
BY workspace_path" to improve performance: in count_sessions_batch build dynamic
placeholders matching workspace_paths.len(), return early with empty vec if
workspace_paths is empty, prepare that single statement (instead of the existing
per-path stmt), bind all workspace_paths as params, iterate over rows to push
(workspace_path, count) into results, and preserve the same
Result<Vec<(String,i64)>, String> error handling.
In `@apps/tauri/src/components/HomeScreen.tsx`:
- Around line 38-51: The dependency array currently calls
dirWorkspaces.map(...).join(",") which allocates on every render and causes
unnecessary closures in fetchSessionCounts (and similarly fetchCommentCounts);
fix by memoizing the derived paths with useMemo (e.g. const workspacePaths =
useMemo(() => dirWorkspaces.map(w => w.path), [dirWorkspaces]) and then use
workspacePaths (or workspacePaths.join(",")) in the dependency arrays, or
alternatively move the mapping inside fetchSessionCounts and depend directly on
dirWorkspaces to avoid the repeated map allocation. Ensure you update both
fetchSessionCounts and fetchCommentCounts to reference the memoized
workspacePaths or dirWorkspaces consistently.
In `@apps/tauri/src/components/WorkspaceCard.tsx`:
- Around line 86-98: The two badges (unresolvedComments and sessionCount) in
WorkspaceCard share identical absolute positioning ("absolute top-1.5
right-1.5") which will cause overlap if both become present; update
WorkspaceCard to either (a) render a single top-right badge container and place
both badges inside it using a vertical or horizontal layout with spacing (e.g.,
a flex column/row with gap) so they don't collide, or (b) give each badge
distinct offsets (e.g., top-1.5 right-1.5 and top-1.5 right-6) so they stack,
and add a brief inline comment above the badge rendering explaining the choice;
ensure you reference and conditionally render unresolvedComments and
sessionCount inside that adjusted container.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/tauri/src-tauri/src/lib.rsapps/tauri/src-tauri/src/sessions.rsapps/tauri/src-tauri/tauri.conf.jsonapps/tauri/src/SettingsApp.tsxapps/tauri/src/components/HomeScreen.tsxapps/tauri/src/components/MarkdownViewer.tsxapps/tauri/src/components/TerminalInput.tsxapps/tauri/src/components/TerminalMessage.tsxapps/tauri/src/components/WorkspaceCard.tsxapps/tauri/src/components/settings/WhisperSettings.tsxapps/tauri/src/components/ui/tabs.tsxapps/tauri/src/index.css
- Collapse review panel visually on plan approval via reviewRef - Optimize count_sessions_batch to single SQL query with IN clause - Memoize workspace paths in HomeScreen to avoid render allocations
Review feedback addressed in e7a7d351. MarkdownViewer.tsx — 2. sessions.rs — 3. HomeScreen.tsx — dependency array allocations (nitpick) 4. WorkspaceCard.tsx — badge positioning (nitpick) 5. tabs.tsx — |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src-tauri/src/sessions.rs`:
- Around line 177-187: count_sessions_batch currently builds one SQL statement
with one placeholder per workspace path which can exceed SQLite's bind-parameter
limit; fix by chunking workspace_paths into batches sized to the connection's
variable limit (use conn.limit(rusqlite::Limit::SQLITE_LIMIT_VARIABLE_NUMBER)
and fall back to 999 if None), for each chunk build the placeholders, prepare
and execute the SELECT ... IN (...) query (using the same logic in
count_sessions_batch but per chunk), collect each chunk's (workspace_path,
count) rows into a HashMap keyed by workspace_path summing counts across chunks,
then return the aggregated Vec<(String, i64)>; ensure you construct params for
each chunk and re-prepare the statement per chunk (refer to function name
count_sessions_batch and use rusqlite::Limit::SQLITE_LIMIT_VARIABLE_NUMBER).
In `@apps/tauri/src/components/MarkdownViewer.tsx`:
- Around line 205-206: The close-on-approve is reverted because the effect that
expands the panel (condition embedded && phase === "reviewing") runs on every
render and can reopen immediately after reviewRef.current?.collapse() and
review.setIsPanelOpen(false); to fix this, change the effect that opens the
panel (the one testing embedded && phase === "reviewing") to only trigger when
entering the reviewing phase by comparing current phase to a stored previous
phase (e.g., prevPhaseRef) or by checking prevPhase !== "reviewing" before
calling the open logic; update the prevPhaseRef after handling so that functions
like reviewRef.current?.collapse() and review.setIsPanelOpen(false) are not
immediately undone.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/tauri/src-tauri/src/sessions.rsapps/tauri/src/components/HomeScreen.tsxapps/tauri/src/components/MarkdownViewer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/tauri/src/components/HomeScreen.tsx
- Chunk workspace_paths into batches of 999 in count_sessions_batch to stay under SQLite's SQLITE_MAX_VARIABLE_NUMBER limit - Track previous phase with a ref so the useEffect that expands the review panel only triggers on transition into "reviewing", preventing it from immediately reverting the close-on-approve
Summary
count_workspace_sessionsTauri command), matching the existing comment count badge patternoverscroll-behavior: none; close review panel automatically on plan approvalTest plan
Summary by CodeRabbit
New Features
Bug Fixes
Style